Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ripley numpy #54

Merged
merged 68 commits into from
Jul 4, 2020
Merged

Ripley numpy #54

merged 68 commits into from
Jul 4, 2020

Conversation

ljwolf
Copy link
Member

@ljwolf ljwolf commented May 4, 2020

migration of pysal/esda#115

@ljwolf ljwolf marked this pull request as draft May 4, 2020 17:15
@ljwolf
Copy link
Member Author

ljwolf commented May 10, 2020

OK, the notebook showing the translation from the old to new API is chugging along is chugging along.

I'd still like to

  1. document how do you specify tests using a given hull, either the types supported by default or a pre-sepcified hull polygon. This can be done by the end of the notebook, in the virginia example
  2. check p-values for validity. I need to double check the logic
  3. code a hand-computable example for the validity tests.

@ljwolf
Copy link
Member Author

ljwolf commented Jun 1, 2020

I've completed tests for all the helper functions, but still need to write the correctness tests.

I also need to document the hulls.

@ljwolf
Copy link
Member Author

ljwolf commented Jun 2, 2020

Tests are now in place and they're pretty comprehensive (if the backlog of commits is anything to show for it :)

I just need to document the hull logic in a notebook & then we're done.

@ljwolf
Copy link
Member Author

ljwolf commented Jun 11, 2020

Locally, this works for all the tests, and fails for some of the Knox statistics I didn't touch... not sure what's up there? We'll have to see in travis, but I think this is rtg. Documentation can be added later to beef up the info on the simulator alternatives.

While we should deprecate those (I think) I'm much less interested in doing so because they're performant.

@ljwolf
Copy link
Member Author

ljwolf commented Jun 11, 2020

OK, well, I'm not sure how to get the specification correct for travis, but tests pass locally aside from the Knox stuff (which I haven't touched). This is ready for review.

@ljwolf ljwolf marked this pull request as ready for review June 11, 2020 21:13
Copy link
Member

@sjsrey sjsrey left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is excellent.

Locally everything passes, except the Knox test as you are finding.

I can try to move forward with debugging the travis issues.

@sjsrey
Copy link
Member

sjsrey commented Jun 28, 2020

OK, well, I'm not sure how to get the specification correct for travis, but tests pass locally aside from the Knox stuff (which I haven't touched). This is ready for review.

See if this solution addresses the travis issues.

@sjsrey
Copy link
Member

sjsrey commented Jul 4, 2020

@ljwolf do you think this is the way we should go on the testing? Yesterday at the dev meeting we briefly mentioned this but I wanted to circle back in case it got lost in the flurry of activity.

If it is the way forward, please merge that pr into your branch, which will update this pr and i think have the tests pass.

@ljwolf
Copy link
Member Author

ljwolf commented Jul 4, 2020

Yes! Sorry, took the recommendation and merged it! I forgot about it when tinkering around today.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants